Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add yaml linter github action #99

Merged
merged 1 commit into from
May 7, 2024
Merged

Add yaml linter github action #99

merged 1 commit into from
May 7, 2024

Conversation

bennerv
Copy link
Contributor

@bennerv bennerv commented May 7, 2024 •
edited
Loading

What this PR does

Before this PR:

  • no yaml linter github action

After this PR:

  • yaml linter github action

Special notes for your reviewer

Checklist

This checklist is not enforcing, but it's a reminder of items that could be relevant to every PR.
Approvers are expected to review this list.

  • PR: The PR description is expressive enough and will help future contributors
  • Code: Write code that humans can understand and Keep it simple
  • Refactor: You have left the code cleaner than you found it (Boy Scout Rule)
  • Upgrade: Impact of this change on upgrade flows was considered and addressed if required
  • Deployment: The deployment process was considered and addressed if required
  • Testing: New code requires new unit tests.
  • Documentation: Is the documentation updated? Either in the doc located in focus area, in the README or in the code itself.
  • Customers: Is this change affecting customers? Is the release plan considered?

@bennerv bennerv force-pushed the add-yaml-linter branch 4 times, most recently from 3e7ebde to d2f63b6 Compare May 7, 2024 01:23
@bennerv bennerv force-pushed the add-yaml-linter branch 4 times, most recently from e293024 to 79f7360 Compare May 7, 2024 01:43
@bennerv bennerv force-pushed the add-yaml-linter branch from 79f7360 to 22af926 Compare May 7, 2024 01:50
mbarnes
mbarnes approved these changes May 7, 2024
View reviewed changes
Copy link
Collaborator

@mbarnes mbarnes left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like this was more or less copied from ARO-RP.

Approved but question: do we want/need the .yaml-lint.yml configuration too?

@s-amann s-amann merged commit d10a53c into Azure:main May 7, 2024
3 checks passed
@bennerv bennerv deleted the add-yaml-linter branch May 7, 2024 14:08
@bennerv
Copy link
Contributor Author

bennerv commented May 7, 2024

Approved but question: do we want/need the .yaml-lint.yml configuration too?

The .yaml-lint.yml config is for the config for the YAML linter. We specifically need the check-keys: false config so it doesn't complain about our github actions that have "truthy-like" values (e.g. on, off, yes, no, ...)

on:   # <--------- this ------
    pull_request:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@s-amann s-amann s-amann approved these changes

@mbarnes mbarnes mbarnes approved these changes

@UlrichSchlueter UlrichSchlueter Awaiting requested review from UlrichSchlueter UlrichSchlueter is a code owner

@jharrington22 jharrington22 Awaiting requested review from jharrington22 jharrington22 is a code owner

@mjlshen mjlshen Awaiting requested review from mjlshen

@petrkotas petrkotas Awaiting requested review from petrkotas

@AldoFusterTurpin AldoFusterTurpin Awaiting requested review from AldoFusterTurpin

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@bennerv @mbarnes @s-amann